Extract getContributionBalance function, use that rather than wrapper… #13151
Conversation
|
(Standard links)
|
eileenmcnaughton
force-pushed
the
payment_function
branch
3 times, most recently
from
1a5edba to
238c918
Compare
… function from Contribution Main class
eileenmcnaughton
force-pushed
the
payment_function
branch
from
238c918 to
26085ea
Compare
| AND ft.status_id IN ({$statusId}, {$refundStatusId}) | ||
| "; | ||
|
|
||
| $ftTotalAmt = CRM_Core_DAO::singleValueQuery($sqlFtTotalAmt); |
There was a problem hiding this comment.
you can further optimize it via:
$ftTotalAmt = CRM_Core_BAO_FinancialTrxn::getTotalPayments($contributionId);There was a problem hiding this comment.
@monishdeb ok - wanna do that as a follow up?
There was a problem hiding this comment.
@monishdeb I just revisited this to see if I should make this suggested change in this PR (now it's Monday :-))
But there is a minor change between this and the function you suggest calling - this function looks at completed & refunded whereas that one only looks at completed.
The getTotalPayments function is called from 2 places & my feeling is that those places would be better served by also looking at refunds - per this - but I think we should analyse that so a follow up PR makes more sense. I think @jitendrapurohit had actually suggested changing one of those places in his recent PR
|
@monishdeb can we merge this? |
| @@ -494,9 +478,6 @@ public static function getPartialPaymentWithType($entityId, $entityName = 'parti | |||
| elseif ($paymentVal > 0) { | |||
| $value['amount_owed'] = $paymentVal; | |||
| } | |||
| elseif ($lineItemTotal == $ftTotalAmt) { | |||
| $value['full_paid'] = $ftTotalAmt; | |||
| } | |||
There was a problem hiding this comment.
lol and I didn't relaised it's not used anwhere in Civi but still we are having a separate attribute.
$ grep -irn "full_paid" CRM/
CRM//Core/BAO/FinancialTrxn.php:498: $value['full_paid'] = $ftTotalAmt;
Agree with this change
| @@ -207,16 +207,15 @@ public function testAddPartialPayment() { | |||
| $amtPaid = 60; | |||
| $balance = $feeAmt - $amtPaid; | |||
| $result = $this->addParticipantWithPayment($feeAmt, $amtPaid); | |||
| extract($result); | |||
| $paymentInfo = CRM_Contribute_BAO_Contribution::getPaymentInfo($participant['id'], 'event'); | |||
| $paymentInfo = CRM_Contribute_BAO_Contribution::getPaymentInfo($result['participant']['id'], 'event'); | |||
Overview
Minor code cleanup to extract the guts of an overloaded function so just the relevant part can be called. These are internal functions only.
Before
Complex function
function getPartialPaymentWithTypedoing multiple thingsAfter
Get balance part callable by itself
Technical Details
@monishdeb @pradpnayak this function is only called from a few places and I couldn't find any evidence that the 'full_paid' parameter is ever used so I dropped that. I feel like this 'getByType' stuff is a bit silly & we should determine that before calling it but for now I haven't touched that.
I also updated the payment only bit from the contribution form to call this function - this came off trying to understand https://github.com/civicrm/civicrm-core/pull/12319/files#diff-e59ea24da123edeec35a0c498d02874fR1322 & feeling that we just need to know the amount owing whatever the status
Comments